Reviewable status: 0 of 5 files reviewed, all discussions resolved (waiting on @Itay-Tsabary-Starkware)

crates/apollo_infra_utils/Cargo.toml line 30 at r1 (raw file):

assert-json-diff.workspace = true
cached.workspace = true
colored.workspace = true

this is s bugfix - cargo test -p apollo_infra_utils fails without these deps

Code quote:

assert-json-diff.workspace = true
cached.workspace = true
colored.workspace = true

Reviewable status: 0 of 5 files reviewed, all discussions resolved (waiting on @dorimedini-starkware)

crates/apollo_infra_utils/Cargo.toml line 10 at r2 (raw file):

[features]
testing = ["cached", "colored", "dep:assert-json-diff", "socket2", "toml"]

I checked the testing feature dependencies and noticed the inconsistent use of dep: syntax here.
What have we decided regarding this usage?

Code quote:

dep:

Reviewable status: 0 of 5 files reviewed, all discussions resolved (waiting on @dorimedini-starkware)

crates/apollo_infra_utils/Cargo.toml line 10 at r2 (raw file):

I don't think we decided anything

Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware)

crates/blockifier_test_utils/src/cairo_compile.rs line 6 at r2 (raw file):

use std::process::{Command, Output};

use apollo_infra_utils::cairo0_compiler::verify_cairo0_compiler_deps;

What's the motivation for having the compiler utilities part of the apollo_infra crate and not in the blockifier test utils crate? Are there other crates that intend to use it?

Code quote:

use apollo_infra_utils::cairo0_compiler::verify_cairo0_compiler_deps;

Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware and @Itay-Tsabary-Starkware)

crates/blockifier_test_utils/src/cairo_compile.rs line 6 at r2 (raw file):

yes :)

Reviewed 4 of 5 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dorimedini-starkware)

crates/apollo_infra_utils/Cargo.toml line 10 at r2 (raw file):

Consider removing the dep: then for consistency. Anyhow, not blocking.

Reviewed 4 of 5 files at r4, all commit messages.
Reviewable status: 4 of 5 files reviewed, 2 unresolved discussions (waiting on @Itay-Tsabary-Starkware and @TzahiTaub)

crates/apollo_infra_utils/src/cairo0_compiler.rs line 16 at r4 (raw file):

pub const STARKNET_COMPILE_DEPRECATED: &str = "starknet-compile-deprecated";
pub const CAIRO0_COMPILE: &str = "cairo-compile";
pub const EXPECTED_CAIRO0_VERSION: &str = "0.14.0a1";

why is it hardcoded here?

Code quote:

pub const EXPECTED_CAIRO0_VERSION: &str = "0.14.0a1";

crates/apollo_infra_utils/src/cairo0_compiler.rs line 32 at r4 (raw file):

pub fn cairo0_compilers_correct_version() -> Result<(), Cairo0CompilerVersionError> {
    for compiler in [CAIRO0_COMPILE, STARKNET_COMPILE_DEPRECATED] {

why are there now two compilers?

Code quote:

for compiler in [CAIRO0_COMPILE, STARKNET_COMPILE_DEPRECATED] {

Reviewable status: 4 of 5 files reviewed, 2 unresolved discussions (waiting on @amosStarkware, @Itay-Tsabary-Starkware, and @TzahiTaub)

crates/apollo_infra_utils/src/cairo0_compiler.rs line 16 at r4 (raw file):

easier to reference it; it's tested for consistency with the pip requirements in test_cairo0_version_pip_requirements

crates/apollo_infra_utils/src/cairo0_compiler.rs line 32 at r4 (raw file):

one for cairo0 code, and one for cairo0 starknet contracts. they are separate scripts

Reviewed 1 of 5 files at r4, 2 of 2 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @TzahiTaub)